Fix undefined reference to "WebSocket" for NodeJS instances #762
Conversation
|
@EzraBrooks do we want to solve it this way, or do we need to import the websocket class here? |
There was a problem hiding this comment.
Thanks for the PR, @mhosmar-cpr! I missed this bug while writing this shim because we don't have unit tests that cover this code path - do you have time to add one?
I also think that we should probably move this condition one layer down as you and @MatthijsBurgh described - even though it's duplicative, it at least is super clear where the magic number comes from if we do that.
|
I have implemented the change as described. However I am not sure of the potential cost of repeatedly importing Unfortunately I don't have the knowledge or time to implement a unit test for this code path. Please understand. |
|
looping back. FYI I can't merge even with approval. |
|
Running CI and will merge once it passes |
Public API Changes
None
Description
Fixed an undefined reference to
WebSocketin src/core/Ros.js. This is a fairly direct approach. We could also rearrange the logic to referencews.WebSocket.CLOSEDon the NodeJS branch. see the issue.Resolves #756